Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: force exact numeric sum - simplex projection #5

Closed
wants to merge 1 commit into from

Conversation

hopeyen
Copy link

@hopeyen hopeyen commented Apr 17, 2023

To fix the bug discovered in graphprotocol/allocation-optimizer#32

There is a small numeric error to performing simplex projection

x = [2.3133337391432116e16]      # vector to project
σ = 652174.7265297174                # sum (simplex goal)
sum(w) = 652176.0                        # resulted vector sum

To quickly deal with the issue we modify the last element to remove the overshoot

@anirudh2
Copy link
Member

Awesome test case for demonstrating the bug! The implementation doesn't make sense to me though.

w[end] += σ - sum(w) could end up being negative depending on the current value of w[end], which breaks the definition of simplex. Furthermore, it doesn't seem very rigorous to subtract from an arbitrary element in the vector.

It seems like this is a problem of insufficient precision, so I propose that we instead convert to BigFloat and back in the method.

@anirudh2
Copy link
Member

anirudh2 commented Apr 17, 2023

Proposed implementation:

function  simplex(x::AbstractVector{T},  ::Real) where {T<:Real}
    _x = convert(Vector{BigFloat}, x)
    _sigma  = convert(BigFloat,  )
    n = length(_x)
    mu = sort(_x; rev=true)
    rho = maximum((1:n)[ -(cumsum(mu).-_sigma)./(1:n).>zero(BigFloat)])
    theta = (sum(mu[1:rho]) - _sigma) / rho
    _w = max.(_x .- theta, zero(BigFloat))
    w = convert(Vector{T}, _w)
    return w
end

Going to close this PR without merging as the proposed implementation passes @hopeyen's test and stays true to the definition of a simplex projection.

@anirudh2 anirudh2 closed this Apr 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants